fix(training): avoid duplicate packed mask transfers - #1405
Open
JESUSROYETH wants to merge 2 commits into
Open
Conversation
JESUSROYETH
requested review from
Borda,
SkalskiP,
isaacrob and
probicheaux
as code owners
August 25, 2026 12:38
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1405 +/- ##
=======================================
+ Coverage 86% 86% +1%
=======================================
Files 114 114
Lines 14880 14890 +10
=======================================
+ Hits 12753 12845 +92
+ Misses 2127 2045 -82 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Optimizes packed segmentation-target transfers to avoid duplicate CUDA mask allocations while preserving mutation semantics.
Changes:
- Adds direct per-sample device materialization via
PackedTargets.to_list(). - Updates training transfer logic and documentation.
- Adds CPU, CUDA, ownership, and memory regression tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/rfdetr/utilities/tensors.py |
Implements direct destination materialization. |
src/rfdetr/training/module_data.py |
Uses the optimized transfer path. |
tests/utilities/test_tensors.py |
Adds ownership, CUDA, and memory tests. |
tests/training/test_module_data.py |
Verifies transfer-hook routing. |
docs/learn/train/training-parameters.md |
Documents the new transfer behavior. |
CHANGELOG.md |
Records the memory fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+912
to
+919
| def peak_extra(materialise): | ||
| torch.cuda.synchronize() | ||
| torch.cuda.reset_peak_memory_stats() | ||
| out = materialise() | ||
| torch.cuda.synchronize() | ||
| extra = torch.cuda.max_memory_allocated() - torch.cuda.memory_allocated() | ||
| del out | ||
| return extra |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Packed segmentation targets are currently copied to CUDA as complete concatenated fields and then cloned per sample. For masks, that temporarily keeps both the packed CUDA field and the materialised tensors alive.
Changes
torch.cuda.max_memory_allocated(), not just values).transfer_batch_to_devicewasPackedTargets.to()'s only caller; add direct coverage for it (same-device identity return, and a CUDA device change that keeps the batch packed) so it doesn't go untested now that it's unused internally.Memory and throughput
Synthetic mask batches (not COCO-sourced;
repro/cuda_peak_memory.py), pinned, three instance-count/resolution sizes. CUDA peak statistics are reset for each synchronized repetition; "extra" is peak minus the memory retained by the returned tensors.The previous path's extra CUDA peak tracks the mask field size almost exactly (ratio 1.000-1.001 across the three sizes). That matches the described mechanism — the packed CUDA field and the materialised per-sample clones stay alive at the same time, until the packed object goes out of scope. The fixed path's extra peak is zero at each size. Timings are noisy at this scale and are not sold as a speedup either way.
A real COCO segmentation val2017 check repeated the complete 5,000-image traversal in three independent processes (577 packed batches per run). The three summaries were identical: the previous temporary allocation was 18,915,328 bytes at the median, 32,188,416 at p95, and 44,048,384 maximum; the fixed path measured 0 bytes at each point. The previous peak follows the complete packed-field size (18,913,604 median, 43,467,692 maximum).
A
PackedTargetsbatch routes through the sameto_list()call for each field it carries. Detection- and keypoint-only batches were not profiled separately here — their fields are boxes, labels, and small metadata tensors, so the previous duplicate was already negligible for them.Real COCO segmentation input-pipeline throughput, using batch size 8, 16 workers, 20 warmup batches, 200 timed batches, and three independent processes:
The two packed ranges overlap, so no speed difference is claimed between them. Direct materialisation retains a 1.51x median advantage over unpacked loading.
Validation
AGENTS.md's command plus the marker exclusions CI itself adds): 4,421 passed, 77 skipped, 100 deselected. 3 pre-existing failures, unrelated to this diff and present on unmodifieddevelop: stale doctests indocs/hooks/package_version.pyandtests/export/test_onnx_notes.py.developand pass with this change.An initial control with unequal chunk boundaries produced small metric deltas (about 2e-6). Matching the chunk boundaries removed the mismatches, so the cause was batch composition, not the packed transfer itself.